Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
For reference, the overloads were added in #10427 due to @AlexWaygood's comment. Are those concerns not valid anymore? (At first glace at |
|
These overloads shouldn't be necessary at all, and they lead to type checker divergence, but there is a very old bug in |
|
Considering the passing tests and the positive primer output, I'd be willing to accept this change, whether the mypy bug is fixed or not. But I'd like to hear Alex's opinion, since he brought the issue up in the original PR. |
That's probably more so because We even get some very bad cases like |
Then new code will have to use |
|
I'm a bit ill right now but happy to take a look later this week (please ping me again if I forget) |
|
@AlexWaygood Friendly ping. |
|
Diff from mypy_primer, showing the effect of this PR on open source code: rotki (https://github.com/rotki/rotki)
+ rotkehlchen/chain/evm/decoding/aave/v3/decoder.py:520: error: Unused "type: ignore" comment [unused-ignore]
artigraph (https://github.com/artigraph/artigraph)
- tests/arti/internal/test_mappings.py:40: error: No overload variant of "__or__" of "dict" matches argument type "frozendict[Never, int]" [operator]
+ tests/arti/internal/test_mappings.py:40: error: Unsupported operand types for | ("dict[str, int]" and "frozendict[Never, int]") [operator]
- tests/arti/internal/test_mappings.py:40: note: Possible overload variants:
- tests/arti/internal/test_mappings.py:40: note: def __or__(self, dict[str, int], /) -> dict[str, int]
- tests/arti/internal/test_mappings.py:40: note: def [_T1, _T2] __or__(self, dict[_T1, _T2], /) -> dict[str | _T1, int | _T2]
|
See #14283. Sibling PR for #14282.